fix(AppFramework): Adjust types so PHPStan understands them#58098
fix(AppFramework): Adjust types so PHPStan understands them#58098provokateurin wants to merge 1 commit intomasterfrom
Conversation
provokateurin
requested review from
leftybournes and
salmart-dev
and removed request for
a team
|
| * @template S of Http::STATUS_* | ||
| * @template-covariant T of DataResponseType | ||
| * @template H of array<string, mixed> | ||
| * @template-covariant T of array|int|float|string|bool|object|null|\stdClass|\JsonSerializable |
There was a problem hiding this comment.
Is that not the same as mixed 🙃 ?
Also, \stdClass|\JsonSerializable is redundant with object, no?
There was a problem hiding this comment.
Nope, this is the correct type. It only allows what can be serialized to JSON.
There was a problem hiding this comment.
But how can a var be \stdClass without being object? And same for JsonSerializable. For me either object or \stdClass|\JsonSerializable should be removed.
There was a problem hiding this comment.
https://www.php.net/manual/en/language.types.mixed.php The only thing refused in this union is resource.
| use OCP\AppFramework\OCSController; | ||
|
|
||
| /** | ||
| * @psalm-import-type DataResponseType from DataResponse |
There was a problem hiding this comment.
There is no generic equivalent to this psalm instruction?
There was a problem hiding this comment.
It's not that phpstan doesn't understand this, it seems to have problems with the template-covariant referencing the alias.
| * @template-extends BaseResponse<Http::STATUS_*, DataResponseType, array<string, mixed>> | ||
| * @template-covariant T of array|int|float|string|bool|object|null|\stdClass|\JsonSerializable | ||
| * @template-covariant H of array<string, mixed> | ||
| * @template-extends BaseResponse<Http::STATUS_*, array|int|float|string|bool|object|null|\stdClass|\JsonSerializable, array<string, mixed>> |
There was a problem hiding this comment.
| * @template-extends BaseResponse<Http::STATUS_*, array|int|float|string|bool|object|null|\stdClass|\JsonSerializable, array<string, mixed>> | |
| * @template-extends BaseResponse<Http::STATUS_*, T, array<string, mixed>> |
Is that not the same type?
There was a problem hiding this comment.
Back when this was added it didn't work with psalm, but yes that is how it is supposed to be.
|
One so higher levels more adjustments are needed, so I'll wait until I have found all problems. |
Pull request was converted to draft
Signed-off-by: provokateurin <kate@provokateurin.de>
provokateurin
force-pushed
the
fix/appframework/types-phpstan
branch
from
c979a7d to
4f81b36
Compare
3 participants
For nextcloud/groupfolders#4344